ssh-tpm-add: add -c flag for per-use confirmation#114
Merged
Foxboron merged 4 commits intoFoxboron:masterfrom Mar 29, 2026
Merged
ssh-tpm-add: add -c flag for per-use confirmation#114Foxboron merged 4 commits intoFoxboron:masterfrom
Foxboron merged 4 commits intoFoxboron:masterfrom
Conversation
os.Setenv() mutates the agent process environment permanently. When SshAskPass is called with hint="confirm", every subsequent passphrase prompt inherits SSH_ASKPASS_PROMPT=confirm and gets rendered as a yes/no dialog by askpass programs that honour the hint. Set the variable on the subprocess cmd.Env instead so it is scoped to the single invocation. This was latent because AskPermission() had no callers yet, but becomes observable once confirm-before-use is wired up in a later commit.
The constraint parser in gocrypto.go was scaffolded but left commented out, so the agentConstrainConfirm byte that MarshalTPMKeyMsg already emits was silently dropped on the receiving side. Uncomment it and hook it into ParseTPMKeyMsg so the flag survives the trip from ssh-tpm-add to the agent. SSHTPMKey grows a ConfirmBeforeUse field and the SSHTPMKeys interface exposes it. Struct literals switch to keyed form so adding the field does not ripple through every constructor call site. No behaviour change yet; the flag is carried but nothing consults it. That comes in the next commit together with the -c flag.
Mirror ssh-add(1) -c: keys added with this flag require the user to approve each signature via SSH_ASKPASS with SSH_ASKPASS_PROMPT=confirm. This gives TPM-sealed keys the same "notify on use" guard that tools like Secretive provide, without inventing a new UI surface. SignWithFlags now looks up the matching key before signing and, if it carries the confirm constraint, calls askpass.AskPermission with the OpenSSH prompt wording. The lookup unwraps certificates on both the request and the stored-key side so a constraint set on the plain key still applies when the client presents a cert wrapping it. AskPermission gains a prompt argument since the fixed "Confirm touch" string was a placeholder from when the function had no callers. The path-argument handling in ssh-tpm-add moves from raw os.Args to flag.Args so -c is not mistaken for a key file.
Owner
|
This looks great, thank you. Would you try write a test for the frontend parts of this so we get an end-to-end test? A reasonable complicated example with |
Contributor
Author
|
Tested together with my new fancy noctalia-plugin: https://github.com/Mic92/noctalia-plugins/tree/main/ssh-askpass |
Exercises the full confirm-before-use path through the agent socket: approve, deny, and the unconstrained case. The askpass stub logs SSH_ASKPASS_PROMPT alongside the prompt text so we can assert the agent both asked and set the OpenSSH confirm hint. Denial is signalled via a sentinel file rather than an env var because the agent is a long-lived background process and does not observe later env mutations in the script.
Contributor
Author
|
@Foxboron here is your test. |
Owner
|
@Mic92 Awesome, thank you :) |
Mic92
added a commit
to Mic92/dotfiles
that referenced
this pull request
Mar 29, 2026
PR #114 (ssh-tpm-add -c confirm-before-use and SSH_ASKPASS_PROMPT leak fix) was merged upstream, so we no longer need to carry our own fork. Pin to the merge commit until the next tagged release lands in nixpkgs. Refs: Foxboron/ssh-tpm-agent#114 Refs: #5177
Mic92
added a commit
to Mic92/dotfiles
that referenced
this pull request
Mar 29, 2026
PR #114 (ssh-tpm-add -c confirm-before-use and SSH_ASKPASS_PROMPT leak fix) was merged upstream, so we no longer need to carry our own fork. Pin to the merge commit until the next tagged release lands in nixpkgs. Refs: Foxboron/ssh-tpm-agent#114 Refs: #5177
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror
ssh-add(1) -c: keys added with-crequire the user to approveeach signature via
SSH_ASKPASSwithSSH_ASKPASS_PROMPT=confirm. Thisgives TPM-sealed keys the same "notify on use" guard that tools like
Secretive provide, without inventing a new UI surface.
The constraint parser in
gocrypto.gowas already scaffolded but leftcommented out, so the
agentConstrainConfirmbyte thatMarshalTPMKeyMsgalready emits was silently dropped on the receiving side. This uncomments
it and hooks it into
ParseTPMKeyMsg, then hasSignWithFlagsconsultthe flag and call
askpass.AskPermissionwith the OpenSSH prompt wording.The certificate lookup unwraps on both sides so a constraint set on the
plain key still applies when the client presents a cert wrapping it.
Also fixes a latent bug where
os.Setenv("SSH_ASKPASS_PROMPT", ...)leaked into subsequent passphrase prompts from the same agent process.
It was dormant because
AskPermissionhad no callers, but becomesobservable the moment confirm-before-use is wired up.
Split into three commits so the env-leak fix stands on its own.